Rust: Remove elements superseded by attribute macro expansions#20770
Rust: Remove elements superseded by attribute macro expansions#20770hvitved merged 2 commits intogithub:mainfrom
Conversation
hvitved
force-pushed
the
rust/attribute-macro-expansion-filter
branch
7 times, most recently
from
e305f21 to
20bb58d
Compare
hvitved
force-pushed
the
rust/attribute-macro-expansion-filter
branch
from
20bb58d to
c81f5f5
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors macro expansion handling in the Rust QL library by consolidating logic into a central MacroExpansion module and introducing the isFromMacroExpansion() predicate to better distinguish between macro arguments and macro-generated code. The changes improve code organization, reduce duplication, and update test expectations to reflect the new behavior.
- Centralized macro expansion logic from
MacroCallImplandPathResolutionintoElementImpl::MacroExpansion - Introduced
isFromMacroExpansion()to exclude macro arguments from expansion checks - Updated queries to use the new predicate where appropriate
- Added custom
toString()for items with attribute macro expansions
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
rust/ql/lib/codeql/rust/elements/internal/ElementImpl.qll |
Adds centralized MacroExpansion module with logic for classifying macro-related elements |
rust/ql/lib/codeql/rust/elements/internal/AstNodeImpl.qll |
Updates isInMacroExpansion() and isFromMacroExpansion() to delegate to MacroExpansion module |
rust/ql/lib/codeql/rust/elements/internal/MacroCallImpl.qll |
Removes isInMacroExpansion() and getATokenTreeNode() logic, now in ElementImpl |
rust/ql/lib/codeql/rust/elements/internal/ItemImpl.qll |
Adds custom toString() for items with attribute macro expansions |
rust/ql/lib/codeql/rust/elements/internal/LocatableImpl.qll |
Updates to use new getImmediatelyEnclosingMacroInvocation() helper |
rust/ql/lib/codeql/rust/internal/PathResolution.qll |
Simplifies ItemNode constructor by removing supersededByAttributeMacroExpansion() |
rust/ql/src/queries/unusedentities/UnusedValue.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
rust/ql/src/queries/unusedentities/UnreachableCode.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
rust/ql/src/queries/unusedentities/UnusedVariable.qll |
Adds TODO comment noting future migration to isFromMacroExpansion() |
rust/ql/lib/utils/test/PathResolutionInlineExpectationsTest.qll |
Simplifies itemAt() by removing unused inMacro parameter |
rust/ql/test/library-tests/variables/variables.ql |
Updates to use isFromMacroExpansion() instead of isInMacroExpansion() |
| Test expectation files | Updates expected test outputs to reflect deduplication and new behavior |
.gitattributes and .generated.list |
Removes ItemImpl.qll from generated file list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
| let a: u16; | ||
| let b: u16 = 2; | ||
| set_value!(a, 1); | ||
| set_value!(a, 1); // $ Alert[rust/unused-value] |
There was a problem hiding this comment.
👍
I'm happy with all of the changes to test results in this PR, and I've made a comment in the DCA run about the results there. I haven't reviewed much of the actual QL (yet).
There was a problem hiding this comment.
Update: happy with the DCA results as well (following some investigation).
geoffw0
left a comment
geoffw0
left a comment
There was a problem hiding this comment.
Code looks good, though its complex enough I think we're leaning on test coverage for confidence here. And test results / DCA LGTM.
2 participants
Follows up on #20454 and removes elements superseded by attribute macro expansions entirely from the
Elementtype. We only ever care about the expanded version, which will typically contain a copy of the original definition.DCA looks good: we reduce inconsistencies, increase call resolution rates, and eliminate
rust/unused-variablefalse positives.